Skip to content

Handle case of div with zero dimension when positioning unified hover box#5913

Merged
archmoj merged 14 commits into
masterfrom
unified-hover-with-zero-outer-height
Aug 31, 2021
Merged

Handle case of div with zero dimension when positioning unified hover box#5913
archmoj merged 14 commits into
masterfrom
unified-hover-with-zero-outer-height

Conversation

@archmoj

@archmoj archmoj commented Aug 30, 2021

Copy link
Copy Markdown
Contributor

Fix to bring back unified hover in documentation website.
Regression introduced by #5846 in v2.3.0.

@plotly/plotly_js

@archmoj archmoj added bug something broken status: reviewable regression this used to work labels Aug 30, 2021
@alexcjohnson

Copy link
Copy Markdown
Collaborator

TBH I suspect we could get away without using getBoundingClientRect at all here - and that would avoid some bugs we probably have right now in case the user has the graph inside some sort of transform.

But in the meantime, to be consistent in our usage, couldn't we switch from outerContainer: fullLayout._paperdiv to outerContainer: fullLayout._paper.node() like we have everywhere else this gets used - ie use _paper (the main <svg>, that always seems to have a meaningful size) rather than _paperdiv (its enclosing <div>, that for whatever weird CSS reason sometimes has no size)... also just pass around a node, don't d3.select it here only to .node() it here

Comment thread src/components/fx/hover.js Outdated

@alexcjohnson alexcjohnson left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💃

@archmoj
archmoj merged commit ba14be2 into master Aug 31, 2021
@archmoj
archmoj deleted the unified-hover-with-zero-outer-height branch August 31, 2021 16:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug something broken regression this used to work

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants